Skip to content

feat(review): add /review:quality-gate code-design mode - #8388

Merged
kyle-sexton merged 17 commits into
mainfrom
feat/8329-quality-gate-code-design-mode
Oct 19, 2026
Merged

kyle-sexton merged 17 commits into
mainfrom
feat/8329-quality-gate-code-design-mode

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Closes #8329

Summary

Adds a code-design mode to /review:quality-gate. It loads the consumer's review criteria through the standards contract and falls back to small bundled criteria when the consumer has none. It rates Duplicated Code that the diff introduces with no shared source or sync check as CRITICAL, naming the repeated fact and the later change that would make the copies diverge; every other repetition is SUGGESTION, as is Markdown repetition and anything under Quality bar: prototype. A consumer standard that contradicts a bundled default wins, and the report says Overrides bundled default <principle> per <standards file>. A missing Quality bar: line is treated as production, and the report says so.

Behavior change: a code-design CRITICAL is a merge blocker in unattended lanes, like any other CRITICAL the gate reports (approved as plan item DA-A).

Fix

  • plugins/review/skills/quality-gate/context/code-design.md (new): bundled criteria, the cascade rule, the CRITICAL exemptions (contract-restatement markers, declared registries, generated files, fixtures, rename diffs; no file-count threshold), consumer-criteria loading and the reviewer brief. The mode always dispatches a fresh-context review:brief-reviewer.
  • SKILL.md: code-design in the description, argument hint and router table.
  • context/phase-boundary.md: code-design reads --base and --quality-bar, and its brief carries the consumer and bundled criteria paths. Built on refactor(review): let /review:quality-gate take phase-boundary inputs (worktree, base SHA, quality bar, report-only) #8327's merged phase-boundary inputs, including its cause-based out-of-phase rule.
  • Five eval cases under plugins/review/evals/qg-code-design-* and a review changelog fragment.

Verification

  • bash scripts/check-changed-skills.sh origin/main: PASS (0 errors, 7 advisory warnings).

  • bash scripts/validate-plugins.sh --only review: all manifests and the catalog validated, 0 failing tests.

  • bash plugins/review/tests/change-set-block.test.sh: all cases passed.

  • python3 -I -B plugins/evals/skills/validate/scripts/validate-cases.py plugins/review/evals: no FAIL (WARN only).

  • Scaffolded-standards probe: the qg-code-design-override scaffold writes docs/standards/service-boundaries.md, and its emits-override-line grader, which matches that path in the output, passed in the with-plugin runs, so a docs/standards/ file written by scaffold_script is readable in an eval run.

  • Paid evals: claude plugin eval, plugin vs no-plugin, 3 runs each, --scaffold, judge opus, threshold 0.8.

    Case With plugin Without Delta
    qg-code-design-cascade 1.0 0.33 +0.67
    qg-code-design-sanctioned 1.0 0.67 +0.33
    qg-code-design-markdown 1.0 0.25 +0.75
    qg-code-design-prototype 1.0 0.25 +0.75
    qg-code-design-override 0.92 (2/3 pass; one run tripped no-blocking-copy-finding) 0.25 +0.67

    Total cost about $9.8.

  • bash scripts/check-affected.sh --tests --base origin/main on the merged head: exit 0, 8 passed, 1 skipped (SKIP ruff (no changed .py)).

Left out: the one check-affected skip is ruff, which has no Python file in this diff to lint.

Acceptance criteria

  • grep -c "code-design" on the quality-gate SKILL.md is ≥ 2 (it is 4), and context/code-design.md exists.
  • Probe that a docs/standards/ file written by a scaffold_script is readable in an eval run; result recorded under Verification.
  • Eval cases qg-code-design-cascade, -sanctioned, -markdown, -prototype, -override, run with and without the plugin; deltas under Verification.
  • check-changed-skills.sh origin/main exits 0 and check-affected.sh --tests exits 0; its only SKIP line is ruff, since the diff has no .py file.
  • A changelog fragment for the review plugin.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 16 commits October 18, 2026 20:22
Every quality-gate mode takes --worktree, --base, --quality-bar and
--report-only, so an orchestrator can review a phase diff in a worker's
worktree. With a base SHA no pull-request lookup or fetch runs. Adds the
qg-phase-boundary-worktree eval case.

Refs #8327

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The phase-boundary inputs are passed explicitly by an orchestrator, so the
description needs no new trigger phrase.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d grants

Restore the argument-hint to the mode set so it fits the 100-character
budget and matches the lead of the Arguments line; the phase-boundary
flags stay documented in the Arguments line and context/phase-boundary.md.

Remove the allowed-tools entries that put a wildcard before the git
subcommand (git -C * ...). A wildcard there also approves options such as
-c core.fsmonitor=<cmd>, which runs a program, in a skill that reviews
untrusted diffs. Worktree-targeted commands go through normal permission
handling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under --base or --worktree, code mode skips the pr-review-toolkit path
for its code-reviewer fallback, and downstream mode skips the remote
downstream_probe layer and treats the floor as report. Under
--report-only the size-crossing loop reads git diff -z instead of a
written path file. Name the modes --quality-bar applies to, pass spec
text to the spec reviewer, and fail the eval grader only on a finding
in admin.js, not on a note that it predates the base.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Name pr and close-out in the --quality-bar row; brief reviewers with
  each mode's own required inputs; restatement dispatches under --base.
- Under --report-only the standards ladder takes its non-interactive rung.
- code skips size crossings under --worktree; define the report-only base.
- Step 0 skips the PR-exists signal with --base; drop restated upstream.
- Eval: fail on any Write, add a no-Edit grader.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
code-reviewer, security-reviewer and architecture-guardian run only the
change-set commands their brief supplies (worktree path and base SHA),
with no gh lookup, fetch or ls-remote; without one, step 2 is unchanged.
phase-boundary.md says the brief replaces that step. criteria mode under
--base or --worktree dispatches a fresh-context review:brief-reviewer
instead of reviewing inline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Restatement mode now dispatches its batch reviewer under --worktree as
well as --base, so a worktree-only run no longer reviews inline from the
invoking checkout.

The phase-boundary reviewer brief names the worktree's absolute path as
the file-read root, and code-reviewer, security-reviewer and
architecture-guardian resolve every Read, Grep, Glob and listed
untracked path under a supplied worktree. The advisory line no longer
reads as if per-slice passes the plan's rationale.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the per-command worktree anchoring with one rule: when the brief
supplies a worktree, every command and tool call in the review targets it
(git via git -C, Read/Grep/Glob, untracked paths, and any other command
such as a dependency audit), never the invoking checkout. Stated
identically in code-reviewer, security-reviewer and architecture-guardian,
and in the quality-gate phase-boundary brief for every dispatched
reviewer, review:brief-reviewer included. The downstream probe's
repository root is the worktree path under --worktree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alternation

typos read the [Ss]earch character class as the misspelling "earch" and
failed lint-repo. (?:search|Search) matches the same strings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The mode loads the consumer's review criteria through the standards
resolution ladder, falls back to small bundled criteria (Duplicated Code,
Shotgun Surgery, Speculative Generality), and dispatches a fresh-context
brief-reviewer. Repetition is CRITICAL only when the diff introduces an
unsynced copy in code, naming the fact and the diverging change; Markdown,
cascades and the prototype bar stay SUGGESTION. A contradicting consumer
standard wins and the report names the overridden default; with no
quality bar the review assumes production and says so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Five plugin-eval cases cover the mode's severity rules: an introduced,
unsynced code copy is CRITICAL with the production assumption named; a
declared-registry entry and a file move are not CRITICAL; Markdown
repetition and the prototype bar stay SUGGESTION; a contradicting
docs/standards file produces the override line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The no-critical and repetition-is-suggestion graders now fail only on a
rated finding (a severity table cell, a line that starts with the
severity, a (CRITICAL) or [CRITICAL] label, or Severity: CRITICAL) and
pass a negated mention such as "Not CRITICAL: setup.md is Markdown".
The cascade grader requires the CRITICAL finding to name
FREE_SHIPPING_THRESHOLD or 50, not the word threshold. code-design.md
states that its repetition rule overrides severity.md's "code
duplication" IMPORTANT example.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 19, 2026 03:52
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-19T03:54:25.373748Z c6b5d4c Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 2cfb05fff81155ee66ce6fbc8d800e126aa1b464: 0 finding(s), 0 as inline comments.

Security review: no findings.

The change since c6b5d4c is one line in plugins/review/evals/qg-code-design-cascade/graders/names-production-assumption.md. It rewrites a regex grader pattern: assum\w* becomes (?:assume[sd]?|assuming|assumptions?).

  • Attack surface: None. The file is an eval grader fixture. It holds no secrets, and no command, path, template or SQL construction reads it. It also sets no permissions and has no Actions surface.
  • Regex cost: The pattern uses only bounded [^\n]{0,80} gaps and flat alternations, with no nested or overlapping quantifiers. A crafted model output therefore cannot trigger catastrophic backtracking in the grader.
  • Instruction surface: The edit narrows which words a grader accepts. It does not delete or soften a standing instruction (CLAUDE.md, rules, a skill body, or a hook block list). The instruction-exception-register lens does not apply, and no protected control is removed.

Skip gate 2 applies: the diff has no security-relevant surface. I posted nothing.

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 2cfb05fff81155ee66ce6fbc8d800e126aa1b464: 0 finding(s), 0 as inline comments.

Incremental review of 2cfb05f, which changes one file: plugins/review/evals/qg-code-design-cascade/graders/names-production-assumption.md.

The change replaces assum\w* with (?:assume[sd]?|assuming|assumptions?) in both alternations of the regex. I read the diff and the grader file. I did not run the regex or the eval.

  • Coverage: the new group matches the natural forms a report would use: assume, assumes, assumed, assuming, assumption and assumptions. default\w* and the other alternatives are unchanged.
  • Symmetry: both directions of the pattern, production … assumed and assumed … production, were edited the same way.
  • Syntax: the regex is well-formed, and the i flag is unchanged.
  • Narrowing: the pattern no longer matches stems such as assumpt… beyond those forms. For this grader that only removes loose matches.

I found no correctness or maintainability problems. All other files in the PR are unchanged since the last review at c6b5d4c.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(review): add /review:quality-gate code-design mode with bundled criteria and cascade severity

1 participant